Skip to content

Return errors instead of panicking#13

Open
atombender wants to merge 2 commits intomainfrom
error-returns
Open

Return errors instead of panicking#13
atombender wants to merge 2 commits intomainfrom
error-returns

Conversation

@atombender
Copy link
Member

@atombender atombender commented Jan 2, 2026

This changes the API to return errors instead of panicking. Adds MustApplyPatch() that preserves the panic behaviour.

This is so we can simply catch the error rather than log it to Sentry as a panic.

I've also updated go.mod to 1.25 and run gopls's modernize tool to update interface{} -> any etc.

@atombender atombender requested a review from a team January 2, 2026 16:41
@judofyr
Copy link
Contributor

judofyr commented Jan 5, 2026

Not sure if I see the need for this change. You always have to pass in a patch which corresponds to the base. If it's an incorrect patch you can then get garbage data even if err == nil. It's truly is an exceptional case if you pass in mismatching patches + data. Not sure if it's worth paying the cost of additional bounds checking and error passing instead of just recovering from the panics like this:

func (options *Options) ApplyPatchRecovery(root interface{}, patch Patch) (res interface{}, err error) {
  defer func() {
    if r := recover(); r != nil {
      err = fmt.Errorf("panic recovered: %v", r)
    }
  }()
  res = options.ApplyPatch(root, patch)
}

@atombender
Copy link
Member Author

atombender commented Jan 5, 2026

That seems less clean to me considering (1) panics are slow in Go compared to normal errors, as they need to unwind the stack, and (2) panics are not specific to mismatches. This is intended to handle the specific error we are seeing in production. Panics can still happen for other reasons than mismatch (i.e. actual code bugs).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants